Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/app/prune.go`:
- Around line 84-90: The deletion loop in applyChainDeletion continues after a
failed d.Dumps.Delete call, which can leave an incremental orphaned after a
.meta.json failure. Change the loop so that once the first member deletion
fails, it records the error on ChainOutcome/PruneResult and stops deleting any
older members in that chain, preserving the complete-shorter-chain behavior.

In `@internal/cli/dumps.go`:
- Around line 162-175: The dumps renderer is marking every applied outcome as
fully “pruned” even when `oc.Errors` indicates a partial failure. Update the
logic in `RenderDumpPrune` so the verb only becomes `pruned` for outcomes that
succeeded without errors, and use a different message for partially failed
chains while still listing `oc.Errors`; keep `Apply`, `res.Outcomes`, and
`oc.Pruned`/`oc.Errors` as the key checks.
- Around line 85-109: Validate the CLI retention override values before invoking
app.Prune in the dumps command flow, since the policy built in the CLI can
currently accept invalid negative values from flags. Add upfront checks around
the override handling near resolveRetentionPolicy and the
keepLastSet/maxAgeSet/gfs*Set assignments so destructive pruning fails fast with
a clear error instead of passing an undefined policy into app.Prune. Keep the
validation aligned with the existing config-backed retention rules and ensure
invalid overrides are rejected before buildDeps and app.Prune are called.

In `@internal/config/config.go`:
- Around line 61-64: Reject negative retention.max_age values in the config
validation path: in the parsing logic for r.MaxAge, after time.ParseDuration
succeeds, add a check that the resulting duration is not less than zero and
return a validation error if it is. Update the validation in the config reader
around r.MaxAge so it still accepts valid durations but explicitly rejects
signed negatives like -1h, keeping the existing error handling style and the
same retention.max_age validation flow.

In `@internal/dumps/retention.go`:
- Around line 81-90: The chain ordering in GroupChains is only sorted by
newest() after building from a map, so equal timestamps still follow randomized
map iteration order. Update the newest-first sort on Chain (and any matching
sort in Plan) to use a stable secondary key like Chain.Root when newest() ties,
or remove the duplicate Plan sort if GroupChains already guarantees determinism.
Use the existing Chain.newest() comparator and the Root field to make keep_last
and GFS selection deterministic across runs.
- Around line 65-70: GroupChains is currently ordering Chain.Members by Created
alone, which can break the base-first/apply-order contract. Update the
grouping/sorting logic in GroupChains to build each chain’s order from
ParentID/BaseID topology first so the root/base always comes before its
incrementals, and use Created only as a secondary tie-breaker for ties. Keep the
Chain.Members ordering consistent with the documented behavior and the
prune/apply path expectations.

In `@README.md`:
- Around line 184-194: The prune command name in this README example is
incorrect and should match the documented CLI path. Update the retention section
text to use the actual command name from the commands table, `siphon dumps
prune`, and make sure any related mention of dry-run or `--apply` refers to that
same command so users are pointed to the right entrypoint.

---

Nitpick comments:
In `@internal/app/prune_test.go`:
- Around line 178-194: Add a regression in TestPrune_CollectsDeletionFailures
for a multi-member chain: instead of only the single dump case, seed a base ->
inc1 -> inc2 sequence, configure the store to fail deleting inc2.meta.json, and
assert Prune collects the failure without deleting the older parent members. Use
the existing helpers like newRecordingStore, seedDump, Prune, and pruneDeps to
locate and extend the test.

In `@internal/config/retention_test.go`:
- Around line 27-53: Add a test case in TestEffectiveRetention_Precedence to
cover an explicit empty retention override on a profile, using
Config.EffectiveRetention and the existing prod/staging setup as reference.
Create a profile with Retention set to an empty RetentionConfig and assert it
returns a non-nil empty result instead of inheriting Defaults.Retention, while
omitted retention still falls back to defaults. Keep the assertions aligned with
the current contract that a profile block replaces defaults wholesale.